Skip to content

Fixes #22967: move auto-assigned incidents to the Assigned stage - #30594

Open
IceS2 wants to merge 14 commits into
mainfrom
fix-incident-should-move-to-assigned-if-auto-assigned
Open

Fixes #22967: move auto-assigned incidents to the Assigned stage#30594
IceS2 wants to merge 14 commits into
mainfrom
fix-incident-should-move-to-assigned-if-auto-assigned

Conversation

@IceS2

@IceS2 IceS2 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Fixes #22967

Problem

An incident task created on test failure already inherits its assignees from the test case owners (TestCaseResolutionStatusRepository.createIncidentTask), but TestCaseResolutionTaskWorkflow always enters NewStage and nothing ever advances it — the only edges out of NewStage are the ack/assign/resolve transitions a user fires by hand.

Two symptoms follow from that single cause:

  • the incident sits in New even though it has an assignee
  • it renders as unassigned, because IncidentTcrsSyncHandler.buildDetailsForStage only attaches assignee details to Assigned records, and the Incident Manager reads the assignee from testCaseResolutionStatusDetails.assignee on the TCRS time series rather than from the task

Fix

After the incident task is created, if assignees resolved, drive the workflow's own assign transition via resolveTaskWithWorkflow — the same path a manual assignment takes. The assign transition targets InProgress, so it takes the non-terminal branch: the workflow advances to AssignedStage, CreateTask stamps workflowStageId=assigned / status=InProgress, and the resulting postUpdate makes the TCRS sync emit an Assigned record carrying the assignee.

Guarded so it only fires when assignees are non-empty, the task actually sits at stage new, and an assign transition is available. Best-effort: an incident that fails to advance is still a usable incident in New, so it never fails test-result ingestion.

Incidents with no owners are untouched and stay in New.

Stage and transition ids of the workflow move into a new IncidentWorkflowStages holder so the TCRS sync handler and this new path cannot drift apart.

InProgress is already in TaskRepository.OPEN_TASK_STATUSES, so incident de-duplication (getOrCreateIncident) and autoCloseIncident both still find the incident after it leaves Open.

Verification

AutoAssignIncidentIT covers both directions — owned test case reaches assigned/InProgress with the owner on the TCRS Assigned record, unowned test case stays new/Open with no Assigned record.

Confirmed end to end against a locally built stack:

owned    -> stage=assigned, status=InProgress, assignees=[owner...]
            TCRS: Assigned(assignee=owner...), New(assignee=None)
unowned  -> stage=new, status=Open, assignees=[]

Known limitation

SetApprovalAssigneesImpl strips the requester from the assignee list to prevent self-approval and does not add them back for workflow-managed tasks. If the user who triggered the test run is the sole owner, the incident ends up unassigned and stays in New — same as a manual assign behaves today. Not addressed here.

Greptile Summary

Moves newly created incidents with inherited assignees through the workflow’s existing assignment transition.

  • Centralizes incident workflow stage and transition identifiers.
  • Updates TCRS synchronization to use the shared stage identifiers.
  • Adds integration coverage for owned and unowned test cases.
  • Updates Playwright expectations and incident-status helpers for auto-assigned incidents.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java Adds a best-effort workflow assignment transition after creating an incident with inherited assignees.
openmetadata-service/src/main/java/org/openmetadata/service/tasks/IncidentWorkflowStages.java Centralizes identifiers shared by incident workflow drivers and synchronization handlers.
openmetadata-service/src/main/java/org/openmetadata/service/events/lifecycle/handlers/IncidentTcrsSyncHandler.java Replaces duplicated stage literals with the shared incident workflow constants.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/AutoAssignIncidentIT.java Verifies owned incidents become Assigned while unowned incidents remain New.
openmetadata-ui/src/main/resources/ui/playwright/utils/incidentManager.ts Adds a non-mutating helper for checking an incident’s displayed status before opening its details.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/IncidentManager.spec.ts Updates the incident-manager scenario to expect automatic assignment rather than manually acknowledging the incident.
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/DataQualityDashboard.spec.ts Updates the owned test case’s expected incident status from New to Assigned.

Sequence Diagram

sequenceDiagram
  participant Test as Test Result Ingestion
  participant Repo as Resolution Status Repository
  participant Task as Task Repository
  participant Workflow as Incident Workflow
  participant TCRS as TCRS Sync Handler

  Test->>Repo: Create failed test result
  Repo->>Task: Create incident with inherited owners
  alt Incident has assignees
    Repo->>Task: Resolve "assign" transition
    Task->>Workflow: Advance New to Assigned
    Workflow->>Task: "Set stage=assigned, status=InProgress"
    Task->>TCRS: Publish task update
    TCRS->>TCRS: Emit Assigned record with assignee
  else Incident has no assignees
    Task->>TCRS: Publish New-stage task
    TCRS->>TCRS: Emit New record
  end
Loading

Reviews (12): Last reviewed commit: "Merge remote-tracking branch 'origin/fix..." | Re-trigger Greptile

Context used (3)

An incident created on test failure already inherits its assignees from the
test case owners, but the resolution workflow always entered NewStage and
nothing ever advanced it. The incident stayed in New, and because the TCRS
mirror only carries assignee details on Assigned records, the Incident
Manager rendered it as unassigned even though the task had assignees.

Drive the workflow's own "assign" transition right after the incident task
is created when assignees resolved, so the incident lands on the Assigned
stage with the owner attached and the TCRS record carries the assignee.
Incidents without owners are untouched and stay in New.

Stage and transition ids of TestCaseResolutionTaskWorkflow move into
IncidentWorkflowStages so the TCRS sync handler and the new assignment path
cannot drift apart.
@IceS2
IceS2 requested a review from a team as a code owner July 28, 2026 14:49
Copilot AI review requested due to automatic review settings July 28, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added Ingestion safe to test Add this label to run secure Github workflows on PRs labels Jul 28, 2026
Log a warning when an incident has assignees but is not at the New stage,
so a broken assumption about synchronous workflow start surfaces instead of
silently leaving the incident unassigned.

Assert the typed assignee id on the Assigned TCRS record rather than
substring-matching the serialized details blob, which could pass on an
incidental match elsewhere in the payload.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@IceS2
IceS2 added this pull request to the merge queue Jul 30, 2026
@IceS2
IceS2 removed this pull request from the merge queue due to a manual request Jul 30, 2026
IceS2 added 2 commits July 30, 2026 17:18
The incident raised by the pipeline re-run in "Resolving incident & re-run
pipeline" is created after the table has an owner, so it now opens as Assigned
rather than New and the hardcoded expectation no longer holds.

Give acknowledgeTask an initialStatus parameter defaulting to New, and pass
Assigned at that one call site. The other three call sites act on incidents
raised before ownership is claimed and keep the default.
…gned-if-auto-assigned' into fix-incident-should-move-to-assigned-if-auto-assigned
Copilot AI review requested due to automatic review settings July 30, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

table3 is given an owner in beforeAll, so the incident raised by its
Consistency failure is auto-assigned on creation and never opens as New.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The Assigned stage offers only reassign and resolve, so an auto-assigned
incident cannot be acknowledged. The rerun step in "Resolving incident &
re-run pipeline" only needs to confirm a fresh incident exists before checking
the open/closed counts, so assert its status directly.

Adds verifyIncidentStatus for that and restores acknowledgeTask to its original
shape, since every remaining caller acts on an incident raised before the table
gained an owner.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

IceS2 added 2 commits July 31, 2026 16:13
Callers go on to openIncidentTaskTab, whose Incident tab only exists on the
test case details page. acknowledgeTask navigated there as a side effect;
verifyIncidentStatus replaced it at one call site and stopped short, leaving
the run on the table's Data Quality tab.
…gned-if-auto-assigned' into fix-incident-should-move-to-assigned-if-auto-assigned

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 2 resolved / 3 findings

Adds automatic workflow advancement for newly created incidents with inherited assignees and centralizes stage identifiers. Consider addressing the transaction handling in persistRecord to prevent nested transaction overhead.

💡 Bug: persistRecord opens its own transaction inside @Transaction storeInternal

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:299-313 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:211-213

persistRecord wraps the insert + upsertIncident + storeRelationship in DeadlockRetry.execute(() -> Entity.getJdbi().inTransaction(...)). It is called both from syncFromTask (no ambient transaction — correct) and from storeInternal, which is annotated @Transaction (and reached via the @Transaction createNewRecord). JDBI3's Jdbi.inTransaction on the core Jdbi instance opens a fresh handle/connection; if the outer @Transaction is effective on these repositories, the inner block would commit on a separate connection independently of the outer transaction, so a later failure in the outer transaction would not roll back the persisted TCRS row/incident/relationship (and each write consumes two pooled connections). If @Transaction is inert on these plain repository objects, the behavior is correct and intentional. Please confirm the outer annotation is not active for this call path, or route the inner writes onto the ambient handle rather than a new inTransaction.

✅ 2 resolved
Edge Case: Auto-advance silently no-ops if workflow start is async

📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:586-600
advanceAutoAssignedIncident is called synchronously right after createInternal and only fires when the re-fetched task is already at stage new with a populated assign transition (canAdvanceToAssignedStage). This relies on triggerWorkflowManagedTask (Flowable triggerByKey) having synchronously driven the instance to the new user-task stage and persisted workflowStageId/availableTransitions before createInternal returns. If the workflow engine is configured to run asynchronously (async job executor), the task is still at pending-workflow-start at this point, canAdvanceToAssignedStage returns false, and the incident silently stays in New with no error — the fix simply does nothing. Consider documenting/asserting this synchronous-execution assumption, or re-driving the assign after the workflow reaches new via the lifecycle event path rather than inline.

Quality: Fragile assignee assertion via details.toString().contains()

📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/AutoAssignIncidentIT.java:105-107
The assigned-record assertion checks assigned.getTestCaseResolutionStatusDetails().toString().contains(owner.getName()), matching against the toString() of the whole details object. This is brittle: it can pass on a coincidental substring match anywhere in the serialized blob (e.g. updatedBy, comment) and does not verify the owner is actually in the assignee list. Assert on the typed assignee field of testCaseResolutionStatusDetails (e.g. that its assignee id equals the owner id) for an unambiguous check.

🤖 Prompt for agents
Code Review: Adds automatic workflow advancement for newly created incidents with inherited assignees and centralizes stage identifiers. Consider addressing the transaction handling in persistRecord to prevent nested transaction overhead.

1. 💡 Bug: persistRecord opens its own transaction inside @Transaction storeInternal
   Files: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:299-313, openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/TestCaseResolutionStatusRepository.java:211-213

   `persistRecord` wraps the insert + `upsertIncident` + `storeRelationship` in `DeadlockRetry.execute(() -> Entity.getJdbi().inTransaction(...))`. It is called both from `syncFromTask` (no ambient transaction — correct) and from `storeInternal`, which is annotated `@Transaction` (and reached via the `@Transaction` `createNewRecord`). JDBI3's `Jdbi.inTransaction` on the core `Jdbi` instance opens a fresh handle/connection; if the outer `@Transaction` is effective on these repositories, the inner block would commit on a separate connection independently of the outer transaction, so a later failure in the outer transaction would not roll back the persisted TCRS row/incident/relationship (and each write consumes two pooled connections). If `@Transaction` is inert on these plain repository objects, the behavior is correct and intentional. Please confirm the outer annotation is not active for this call path, or route the inner writes onto the ambient handle rather than a new `inTransaction`.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automating Incident Assignee/Creation

3 participants